refactor(accessibility): Add accessibility features to Color Picker hue slider #5656
Conversation
|
Hey @pat270 @matuzalemsteles |
| @@ -79,6 +79,7 @@ const ClayColorPickerCustom = ({ | |||
|
|
|||
| {showPalette && ( | |||
| <button | |||
| aria-label={editorActive ? 'close' : 'custom color'} | |||
There was a problem hiding this comment.
This was something out of scope but since there wasn't any description on this button, I added an aria-label.
Is there any language key we can use on this?
There was a problem hiding this comment.
@ilzamcmed we don't have a language key because Clay is distributed outside of DXP too so we don't have Liferay.Language.get for example which is just processed in SSR instead we always expose an API to configure aria-labels, in that if we are adopting a pattern of exposing a messages API being the object with keys that are related to language, we can do the same here.
| @@ -79,6 +79,7 @@ const ClayColorPickerCustom = ({ | |||
|
|
|||
| {showPalette && ( | |||
| <button | |||
| aria-label={editorActive ? 'close' : 'custom color'} | |||
There was a problem hiding this comment.
@ilzamcmed we don't have a language key because Clay is distributed outside of DXP too so we don't have Liferay.Language.get for example which is just processed in SSR instead we always expose an API to configure aria-labels, in that if we are adopting a pattern of exposing a messages API being the object with keys that are related to language, we can do the same here.
| style={{ | ||
| background: `hsl(${value}, 100%, 50%)`, | ||
| left: x - 7, | ||
| <div className="clay-color-form-group"> |
There was a problem hiding this comment.
I think we don't need it here for this element.
ilzamcmed
changed the title
|
@ilzamcmed apparently the branch is out of date or something happened that commits are coming from master. Could you see it? it also has some typescript errors. |
ilzamcmed
force-pushed
the
LPS-190649
branch
3 times, most recently
from
8a3ec0a
to
ec82194
Compare
| userEvent.keyboard('[ArrowRight]'); | ||
|
|
||
| expect(handleColorsChange).toBeCalledTimes(2); | ||
| expect(handleValueChange).toBeCalledTimes(2); |
There was a problem hiding this comment.
Hey @matuzalemsteles
The reason I had to add two checks on this test was because to set focus on the slider, I had to use a click and that triggers the onChange function and when i press the arrow key, it triggers again.
|
|
||
| userEvent.keyboard('[ArrowLeft]'); | ||
| expect(handleColorsChange).toBeCalledTimes(2); | ||
| expect(handleValueChange).toBeCalledTimes(2); |
… unwanted effects on children
Jira ticket: https://liferay.atlassian.net/browse/LPS-190649